Add tooltip for sourceChannelId map#217
Add tooltip for sourceChannelId map#217mgaffigan wants to merge 2 commits intoOpenIntegrationEngine:mainfrom
Conversation
Signed-off-by: Mitch Gaffigan <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR adds tooltip functionality to the metadata mappings table in the Message Browser. When hovering over a value in the mappings table where the key is "sourceChannelId", the tooltip displays the resolved channel name instead of just the channel ID, improving usability.
- Introduces a custom
ValueTooltipCellRenderercell renderer for the mappings table value column - Implements channel name resolution by looking up the channel ID in the dashboard status list
- Maintains the existing HTML-disabled behavior for the value column
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowser.java
Outdated
Show resolved
Hide resolved
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowser.java
Show resolved
Hide resolved
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowser.java
Show resolved
Hide resolved
|
As a channel dev - I like this. |
I kept it as a tooltip to avoid false impressions, and to avoid the cost of a lookup. I think it is important that the value displayed should match the actual value. It might be ok with distinguishing appearance, or with a new column, but this seemed the lesser evil. |
…eBrowser.java Fix typo Co-authored-by: Copilot <[email protected]> Signed-off-by: Jon Bartels <[email protected]>
|
Can you click the tooltip to go to the Channel and locate the message |
I wish. I tried adding navigation as a double-click action for |
|
This is a really useful feature. |
|
I suggest merging this in as is. It's a great feature and then you can iterate and add the navigation later (which I think it's a GREAT idea). |
| } | ||
|
|
||
| // Resolve tooltip text for a mapping entry based on key/value | ||
| private String resolveMapValueTooltip(String key, String value) { |
There was a problem hiding this comment.
suggestion: make value an Object instead of String
As currently implemented, all table values are Strings without needing to convert them to a String, however, the underlying table implementation supports arbitrary Objects as values. It would be useful to support other types in this method, like lists of strings or integers, without needing to parse the string to get the values.
In the case of this PR, this should have minimal impact, because the channel id is already a String to begin with.
An obvious case for this would be sourceChannelIds, which is a List<String> of channelIds in a chain of channel writers.
| Component c = super.getTableCellRendererComponent(table, value, isSelected, hasFocus, row, column); | ||
| if (column == 2 /* VALUE_COLUMN_NAME */) { | ||
| String key = String.valueOf(table.getModel().getValueAt(table.convertRowIndexToModel(row), 1)); | ||
| String val = String.valueOf(value); |
There was a problem hiding this comment.
suggestion: do not convert value to String explicitly
As mentioned in the other review comment, the values are already converted to Strings before placing in the table in the current implementation, but they could be placed in the table with the actual Object values which would allow other types to be processed for generating tooltips.
If you are going to keep the explicit String conversion (not my preference) the preferred method in this file seems to be StringUtil.valueOf() for special array handling.
client/src/com/mirth/connect/client/ui/browsers/message/MessageBrowser.java
Show resolved
Hide resolved
| private String resolveMapValueTooltip(String key, String value) { | ||
| // Show channel name when key is sourceChannelId | ||
| if ("sourceChannelId".equals(key)) { | ||
| return parent.status.stream() |
There was a problem hiding this comment.
issue: lookup fails when the source channel is not deployed
The dashboard status only enumerates deployed channels. This lookup should search the full channel list instead.
6 participants
Adds a tooltip to metadata named "sourceChannelId" with the resolved channel name.
Closes #216